Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add iai_callgrind benchmark for measuring memory #64569

Closed

Conversation

arlyon
Copy link
Contributor

@arlyon arlyon commented Apr 16, 2024

No description provided.

@ijjk ijjk added Turbopack Related to Turbopack with Next.js. type: next labels Apr 16, 2024
Copy link
Contributor Author

arlyon commented Apr 16, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @arlyon and the rest of your teammates on Graphite Graphite

@ijjk
Copy link
Member

ijjk commented Apr 16, 2024

Allow CI Workflow Run

  • approve CI run for commit: ee00dad

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@arlyon arlyon changed the title add benches Summary for Pull Request Apr 16, 2024
@arlyon arlyon changed the title Summary for Pull Request Add iai_callgrind benchmark for measuring memory Apr 16, 2024
@arlyon arlyon marked this pull request as ready for review April 16, 2024 16:25
@arlyon arlyon requested review from timneutkens, ijjk, shuding, huozhi and a team as code owners April 16, 2024 16:25
@arlyon arlyon force-pushed the arlyon/memory-benchmark-iai-callgrind branch from 442b6bc to 778aee9 Compare April 16, 2024 17:06
@arlyon arlyon mentioned this pull request Apr 16, 2024
Copy link
Contributor

github-actions bot commented Apr 16, 2024

All broken links are now fixed, thank you!

@arlyon arlyon force-pushed the arlyon/memory-benchmark-iai-callgrind branch from 778aee9 to 9d06c47 Compare April 16, 2024 17:19

fn get_container() -> (turbo_tasks::Vc<ProjectContainer>, tempdir::TempDir) {
let (options, dir) = init_bench(
"https://github.com/shadcn-ui/ui.git",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want a reproducible benchmark, this should have a commit hash associated with it. If you don't want to hardcode it, maybe take the commit hash in as an environment variable?

"https://github.com/shadcn-ui/ui.git",
"pnpm i",
Some("apps/www"),
"{\n \"env\": {},\n \"webpack\": {},\n \"eslint\": {\n \"ignoreDuringBuilds\": false\n },\n \"typescript\": {\n \"ignoreBuildErrors\": false,\n \"tsconfigPath\": \"tsconfig.json\"\n },\n \"distDir\": \".next\",\n \"cleanDistDir\": true,\n \"assetPrefix\": \"\",\n \"cacheMaxMemorySize\": 52428800,\n \"configOrigin\": \"next.config.mjs\",\n \"useFileSystemPublicRoutes\": true,\n \"generateBuildId\": null,\n \"generateEtags\": true,\n \"pageExtensions\": [\n \"tsx\",\n \"ts\",\n \"jsx\",\n \"js\"\n ],\n \"poweredByHeader\": true,\n \"compress\": true,\n \"analyticsId\": \"\",\n \"images\": {\n \"deviceSizes\": [\n 640,\n 750,\n 828,\n 1080,\n 1200,\n 1920,\n 2048,\n 3840\n ],\n \"imageSizes\": [\n 16,\n 32,\n 48,\n 64,\n 96,\n 128,\n 256,\n 384\n ],\n \"path\": \"/_next/image\",\n \"loader\": \"default\",\n \"loaderFile\": \"\",\n \"domains\": [],\n \"disableStaticImages\": false,\n \"minimumCacheTTL\": 60,\n \"formats\": [\n \"image/webp\"\n ],\n \"dangerouslyAllowSVG\": false,\n \"contentSecurityPolicy\": \"script-src 'none'; frame-src 'none'; sandbox;\",\n \"contentDispositionType\": \"inline\",\n \"remotePatterns\": [\n {\n \"protocol\": \"https\",\n \"hostname\": \"avatars.githubusercontent.com\"\n },\n {\n \"protocol\": \"https\",\n \"hostname\": \"images.unsplash.com\"\n }\n ],\n \"unoptimized\": false\n },\n \"devIndicators\": {\n \"buildActivity\": true,\n \"buildActivityPosition\": \"bottom-right\"\n },\n \"onDemandEntries\": {\n \"maxInactiveAge\": 3600000,\n \"pagesBufferLength\": 5\n },\n \"amp\": {\n \"canonicalBase\": \"\"\n },\n \"basePath\": \"\",\n \"sassOptions\": {},\n \"trailingSlash\": false,\n \"i18n\": null,\n \"productionBrowserSourceMaps\": false,\n \"optimizeFonts\": true,\n \"excludeDefaultMomentLocales\": true,\n \"serverRuntimeConfig\": {},\n \"publicRuntimeConfig\": {},\n \"reactProductionProfiling\": false,\n \"reactStrictMode\": true,\n \"httpAgentOptions\": {\n \"keepAlive\": true\n },\n \"outputFileTracing\": true,\n \"staticPageGenerationTimeout\": 60,\n \"swcMinify\": true,\n \"modularizeImports\": {\n \"@mui/icons-material\": {\n \"transform\": \"@mui/icons-material/{{member}}\"\n },\n \"lodash\": {\n \"transform\": \"lodash/{{member}}\"\n }\n },\n \"experimental\": {\n \"prerenderEarlyExit\": false,\n \"serverMinification\": true,\n \"serverSourceMaps\": false,\n \"linkNoTouchStart\": false,\n \"caseSensitiveRoutes\": false,\n \"clientRouterFilter\": true,\n \"clientRouterFilterRedirects\": false,\n \"fetchCacheKeyPrefix\": \"\",\n \"middlewarePrefetch\": \"flexible\",\n \"optimisticClientCache\": true,\n \"manualClientBasePath\": false,\n \"cpus\": 9,\n \"memoryBasedWorkersCount\": false,\n \"isrFlushToDisk\": true,\n \"workerThreads\": false,\n \"optimizeCss\": false,\n \"nextScriptWorkers\": false,\n \"scrollRestoration\": false,\n \"externalDir\": false,\n \"disableOptimizedLoading\": false,\n \"gzipSize\": true,\n \"craCompat\": false,\n \"esmExternals\": true,\n \"fullySpecified\": false,\n \"outputFileTracingRoot\": \"/Users/arlyon/Programming/ui\",\n \"swcTraceProfiling\": false,\n \"forceSwcTransforms\": false,\n \"largePageDataBytes\": 128000,\n \"adjustFontFallbacks\": false,\n \"adjustFontFallbacksWithSizeAdjust\": false,\n \"typedRoutes\": false,\n \"instrumentationHook\": false,\n \"bundlePagesExternals\": false,\n \"parallelServerCompiles\": false,\n \"parallelServerBuildTraces\": false,\n \"ppr\": false,\n \"missingSuspenseWithCSRBailout\": true,\n \"optimizeServerReact\": true,\n \"useEarlyImport\": false,\n \"staleTimes\": {\n \"dynamic\": 30,\n \"static\": 300\n },\n \"optimizePackageImports\": [\n \"lucide-react\",\n \"date-fns\",\n \"lodash-es\",\n \"ramda\",\n \"antd\",\n \"react-bootstrap\",\n \"ahooks\",\n \"@ant-design/icons\",\n \"@headlessui/react\",\n \"@headlessui-float/react\",\n \"@heroicons/react/20/solid\",\n \"@heroicons/react/24/solid\",\n \"@heroicons/react/24/outline\",\n \"@visx/visx\",\n \"@tremor/react\",\n \"rxjs\",\n \"@mui/material\",\n \"@mui/icons-material\",\n \"recharts\",\n \"react-use\",\n \"@material-ui/core\",\n \"@material-ui/icons\",\n \"@tabler/icons-react\",\n \"mui-core\",\n \"react-icons/ai\",\n \"react-icons/bi\",\n \"react-icons/bs\",\n \"react-icons/cg\",\n \"react-icons/ci\",\n \"react-icons/di\",\n \"react-icons/fa\",\n \"react-icons/fa6\",\n \"react-icons/fc\",\n \"react-icons/fi\",\n \"react-icons/gi\",\n \"react-icons/go\",\n \"react-icons/gr\",\n \"react-icons/hi\",\n \"react-icons/hi2\",\n \"react-icons/im\",\n \"react-icons/io\",\n \"react-icons/io5\",\n \"react-icons/lia\",\n \"react-icons/lib\",\n \"react-icons/lu\",\n \"react-icons/md\",\n \"react-icons/pi\",\n \"react-icons/ri\",\n \"react-icons/rx\",\n \"react-icons/si\",\n \"react-icons/sl\",\n \"react-icons/tb\",\n \"react-icons/tfi\",\n \"react-icons/ti\",\n \"react-icons/vsc\",\n \"react-icons/wi\"\n ]\n },\n \"configFile\": \"/Users/arlyon/Programming/ui/apps/www/next.config.mjs\",\n \"configFileName\": \"next.config.mjs\",\n \"_originalRedirects\": [\n {\n \"source\": \"/components\",\n \"destination\": \"/docs/components/accordion\",\n \"permanent\": true\n },\n {\n \"source\": \"/docs/components\",\n \"destination\": \"/docs/components/accordion\",\n \"permanent\": true\n },\n {\n \"source\": \"/examples\",\n \"destination\": \"/examples/mail\",\n \"permanent\": false\n },\n {\n \"source\": \"/docs/primitives/:path*\",\n \"destination\": \"/docs/components/:path*\",\n \"permanent\": true\n },\n {\n \"source\": \"/figma\",\n \"destination\": \"/docs/figma\",\n \"permanent\": true\n },\n {\n \"source\": \"/docs/forms\",\n \"destination\": \"/docs/components/form\",\n \"permanent\": false\n },\n {\n \"source\": \"/docs/forms/react-hook-form\",\n \"destination\": \"/docs/components/form\",\n \"permanent\": false\n }\n ],\n \"exportPathMap\": {}\n}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is illegible, and seems like it might be difficult to update. Can we move these long strings into separate json files and pull them in with include_str!()?

}

fn do_in_runtime() -> impl FnOnce(
Pin<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this type can be cleaned up a bit so that it doesn't get split into so many lines:


let rt = tokio::runtime::Builder::new_multi_thread()
.enable_all()
.on_thread_stop(|| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this just be

.on_thread_stop(TurboMalloc::thread_stop)

(and shouldn't clippy detect this? maybe I'm missing something?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There is another place in the codebase where it is initialized without eta reduction so I just kept it consistent.

let entrypoints = container.entrypoints().await.unwrap();

let routes = entrypoints.routes.clone().into_iter().limit(pages);
// .filter(|route| SELECTED_PAGES.contains(&route.0.as_str()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover comment?

.unwrap();

let project_path = package_path
.map(|pp| tmp.path().join(pp))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map_or_else should combine this with the .unwrap_or_else below it.

https://doc.rust-lang.org/std/option/enum.Option.html#method.map_or_else

I thought there was a clippy lint for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don' think clippy runs on benchmark code (or examples).

@arlyon arlyon force-pushed the arlyon/memory-benchmark-iai-callgrind branch from 9d06c47 to ee00dad Compare April 17, 2024 12:57
@arlyon arlyon closed this Apr 17, 2024
Copy link
Contributor Author

@arlyon arlyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing this as the overhead in recording is too much to be run in CI

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants